Skip to content

Improve BCMath performance and reduce memory usage #14101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
May 1, 2024
Merged

Improve BCMath performance and reduce memory usage #14101

merged 3 commits into from
May 1, 2024

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented May 1, 2024

See individual commits.
Using the benchmark from #14076 I obtain the following results on an i7-4790:

before:

1.9940659999847
2.608274936676
2.8669791221619

after:

1.672737121582
2.3618471622467
2.3474779129028

nielsdos added 3 commits May 1, 2024 15:09
@nielsdos
Avoid double allocation in _bc_new_num_ex
fbc7899 
Since the two allocations are tied together anyway, we can just use a
single allocation. Moreover, this actually seemed like the intention
because the bc_struct allocation already accounted for the length and
scale.
@nielsdos
Merge n_value and n_ptr
7a03c19
@nielsdos
Use an unsigned number for the refcount of bcmath objects
a7d1863
@nielsdos nielsdos requested a review from SakiTakamachi May 1, 2024 13:12
@nielsdos nielsdos requested a review from Girgias as a code owner May 1, 2024 13:12
nielsdos added a commit to nielsdos/php-src that referenced this pull request May 1, 2024
@nielsdos
Use bulk conversion in BCMath of BCD/CHAR where possible
30b6d2f 
On my i7-4790 with benchmark from php#14076, on top of php#14101 I obtain the
following results:

before (with php#14101):
```
1.672737121582
2.3618471622467
2.3474779129028
```

after (with php#14101 + this):
```
1.5878579616547
2.0568618774414
2.0204811096191
```
@nielsdos nielsdos mentioned this pull request May 1, 2024
Merged
Girgias
Girgias approved these changes May 1, 2024
View reviewed changes
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please merge each individual commit (i.e. merge as a rebase instead of squash)

@nielsdos nielsdos merged commit d2d4596 into php:master May 1, 2024
9 of 10 checks passed
nielsdos added a commit that referenced this pull request May 1, 2024
@nielsdos
Use bulk conversion in BCMath of BCD/CHAR where possible (#14103)
0a3ccc0 
On my i7-4790 with benchmark from #14076, on top of #14101 I obtain the
following results:

before (with #14101):
```
1.672737121582
2.3618471622467
2.3474779129028
```

after (with #14101 + this):
```
1.5878579616547
2.0568618774414
2.0204811096191
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Girgias Girgias Girgias approved these changes

@SakiTakamachi SakiTakamachi SakiTakamachi approved these changes

Assignees
No one assigned
Labels
Extension: bcmath
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nielsdos @Girgias @SakiTakamachi